Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UII] Expose advanced file logging config in UI #200274

Merged
merged 12 commits into from
Nov 20, 2024

Conversation

jen-huang
Copy link
Contributor

@jen-huang jen-huang commented Nov 14, 2024

Summary

Resolves #192237. This PR exposes the following Elastic Agent file logging configuration options in the agent policy advanced settings UI:

agent.logging.to_files
agent.logging.files.rotateeverybytes 
agent.logging.files.keepfiles
agent.logging.files.interval
image

This PR also does some clean up on the default values for all these configured advanced settings so that when user has not touched them, the default values do not get written into the agent policy saved object. More info here.

It also fixes adds missing response schemas for the advanced settings.

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

@jen-huang jen-huang added Team:Fleet Team label for Observability Data Collection Fleet team release_note:feature Makes this part of the condensed release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Nov 14, 2024
@jen-huang jen-huang self-assigned this Nov 14, 2024
@jen-huang jen-huang requested a review from a team as a code owner November 14, 2024 23:06
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/fleet --update'
@@ -24,40 +24,19 @@ export function _getSettingsAPISchema(settings: SettingsConfig[]): Props {
if (!setting.api_field) {
return;
}
const defaultValueRes = setting.schema.safeParse(undefined);
const defaultValue = defaultValueRes.success ? defaultValueRes.data : undefined;
if (defaultValue) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question: it's ok to remove the default value but it's not clear to me why . Is that a change in requirement or was it causing some issues?

Copy link
Contributor Author

@jen-huang jen-huang Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@criamico I found that when the default value is present in the API schema, it means that they will get added regardless of what the original request payload is

for example, for this request that updates a single advanced option:

PUT kbn:/api/fleet/agent_policies/<policy id>
{
  "advanced_settings": {
    "agent_logging_to_files": false
  }
}

once it makes it way to the handler, any advanced options with a default value gets added to the update policy payload:

{
  "advanced_settings": {
    "agent_logging_level": "info",
    "agent_logging_to_files": false
  }
}

this means that advanced options that the user did not touch at all gets saved into the agent policy. I don't think this is correct behavior because we prefer to let elastic agent fill in default values rather than dictating it from Fleet. imagine that elastic agent changes the default value of this config to something better in the future, in this case agents enrolled in this policy will not get the updated default value because it got hardcoded into the agent policy due to this behavior.

hope this helps!

@jillguyonnet
Copy link
Contributor

jillguyonnet commented Nov 18, 2024

Tested and looks good. I was able to set negative values through the UI (not the API) though (it saved successfully), is this something that can be fixed?

Screenshot 2024-11-18 at 11 35 11

@jen-huang
Copy link
Contributor Author

Tested and looks good. I was able to set negative values through the UI (not the API) though (it saved successfully), is this something that can be fixed?

@jillguyonnet Fixed!

kibanamachine and others added 3 commits November 19, 2024 22:55
…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/fleet --update'
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #13 / UserActionsList renders list correctly with isExpandable option

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
fleet 1.7MB 1.7MB +2.9KB

History

cc @jen-huang

Copy link
Contributor

@criamico criamico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

Copy link
Contributor

@jillguyonnet jillguyonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the fix 🚀

@jen-huang jen-huang merged commit 0aa63a7 into elastic:main Nov 20, 2024
24 checks passed
@jen-huang jen-huang deleted the feat/adv-logging-config branch November 20, 2024 17:41
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11938833595

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 200274

Questions ?

Please refer to the Backport tool documentation

@jen-huang
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

jen-huang added a commit to jen-huang/kibana that referenced this pull request Nov 20, 2024
## Summary

Resolves [elastic#192237](elastic#192237).
This PR exposes the following Elastic Agent file logging configuration
options in the agent policy advanced settings UI:

```
agent.logging.to_files
agent.logging.files.rotateeverybytes
agent.logging.files.keepfiles
agent.logging.files.interval
```

<img width="1237" alt="image"
src="https://github.com/user-attachments/assets/8de9023c-29a0-4ecf-803a-d8c0c4b87616">

This PR also does some clean up on the default values for all these
configured advanced settings so that when user has not touched them, the
default values do not get written into the agent policy saved object.
[More info
here](elastic#200274 (comment)).

It also fixes adds missing response schemas for the advanced settings.

### Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 0aa63a7)

# Conflicts:
#	oas_docs/bundle.json
#	oas_docs/bundle.serverless.json
#	oas_docs/output/kibana.serverless.yaml
#	oas_docs/output/kibana.yaml
#	x-pack/plugins/fleet/server/routes/agent_policy/index.test.ts
#	x-pack/plugins/fleet/server/services/form_settings/form_settings.ts
#	x-pack/plugins/fleet/server/types/models/agent_policy.ts
jen-huang added a commit that referenced this pull request Nov 20, 2024
)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[UII] Expose advanced file logging config in UI
(#200274)](#200274)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Jen
Huang","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-20T17:41:50Z","message":"[UII]
Expose advanced file logging config in UI (#200274)\n\n##
Summary\r\n\r\nResolves
[#192237](https://github.com/elastic/kibana/issues/192237).\r\nThis PR
exposes the following Elastic Agent file logging
configuration\r\noptions in the agent policy advanced settings
UI:\r\n\r\n```\r\nagent.logging.to_files\r\nagent.logging.files.rotateeverybytes
\r\nagent.logging.files.keepfiles\r\nagent.logging.files.interval\r\n```\r\n\r\n<img
width=\"1237\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/8de9023c-29a0-4ecf-803a-d8c0c4b87616\">\r\n\r\nThis
PR also does some clean up on the default values for all
these\r\nconfigured advanced settings so that when user has not touched
them, the\r\ndefault values do not get written into the agent policy
saved object.\r\n[More
info\r\nhere](https://github.com/elastic/kibana/pull/200274#discussion_r1849142612).\r\n\r\nIt
also fixes adds missing response schemas for the advanced
settings.\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following
conditions. \r\n\r\nReviewers should verify this PR satisfies this list
as well.\r\n\r\n- [x] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"0aa63a7eccea009174db782c57577226ea252bff","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Fleet","v9.0.0","release_note:feature","backport:prev-minor"],"number":200274,"url":"https://github.com/elastic/kibana/pull/200274","mergeCommit":{"message":"[UII]
Expose advanced file logging config in UI (#200274)\n\n##
Summary\r\n\r\nResolves
[#192237](https://github.com/elastic/kibana/issues/192237).\r\nThis PR
exposes the following Elastic Agent file logging
configuration\r\noptions in the agent policy advanced settings
UI:\r\n\r\n```\r\nagent.logging.to_files\r\nagent.logging.files.rotateeverybytes
\r\nagent.logging.files.keepfiles\r\nagent.logging.files.interval\r\n```\r\n\r\n<img
width=\"1237\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/8de9023c-29a0-4ecf-803a-d8c0c4b87616\">\r\n\r\nThis
PR also does some clean up on the default values for all
these\r\nconfigured advanced settings so that when user has not touched
them, the\r\ndefault values do not get written into the agent policy
saved object.\r\n[More
info\r\nhere](https://github.com/elastic/kibana/pull/200274#discussion_r1849142612).\r\n\r\nIt
also fixes adds missing response schemas for the advanced
settings.\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following
conditions. \r\n\r\nReviewers should verify this PR satisfies this list
as well.\r\n\r\n- [x] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"0aa63a7eccea009174db782c57577226ea252bff"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/200274","number":200274,"mergeCommit":{"message":"[UII]
Expose advanced file logging config in UI (#200274)\n\n##
Summary\r\n\r\nResolves
[#192237](https://github.com/elastic/kibana/issues/192237).\r\nThis PR
exposes the following Elastic Agent file logging
configuration\r\noptions in the agent policy advanced settings
UI:\r\n\r\n```\r\nagent.logging.to_files\r\nagent.logging.files.rotateeverybytes
\r\nagent.logging.files.keepfiles\r\nagent.logging.files.interval\r\n```\r\n\r\n<img
width=\"1237\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/8de9023c-29a0-4ecf-803a-d8c0c4b87616\">\r\n\r\nThis
PR also does some clean up on the default values for all
these\r\nconfigured advanced settings so that when user has not touched
them, the\r\ndefault values do not get written into the agent policy
saved object.\r\n[More
info\r\nhere](https://github.com/elastic/kibana/pull/200274#discussion_r1849142612).\r\n\r\nIt
also fixes adds missing response schemas for the advanced
settings.\r\n\r\n### Checklist\r\n\r\nCheck the PR satisfies following
conditions. \r\n\r\nReviewers should verify this PR satisfies this list
as well.\r\n\r\n- [x] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"0aa63a7eccea009174db782c57577226ea252bff"}}]}]
BACKPORT-->
TattdCodeMonkey pushed a commit to TattdCodeMonkey/kibana that referenced this pull request Nov 21, 2024
## Summary

Resolves [elastic#192237](elastic#192237).
This PR exposes the following Elastic Agent file logging configuration
options in the agent policy advanced settings UI:

```
agent.logging.to_files
agent.logging.files.rotateeverybytes 
agent.logging.files.keepfiles
agent.logging.files.interval
```

<img width="1237" alt="image"
src="https://github.com/user-attachments/assets/8de9023c-29a0-4ecf-803a-d8c0c4b87616">

This PR also does some clean up on the default values for all these
configured advanced settings so that when user has not touched them, the
default values do not get written into the agent policy saved object.
[More info
here](elastic#200274 (comment)).

It also fixes adds missing response schemas for the advanced settings.

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Nov 26, 2024
## Summary

Resolves [elastic#192237](elastic#192237).
This PR exposes the following Elastic Agent file logging configuration
options in the agent policy advanced settings UI:

```
agent.logging.to_files
agent.logging.files.rotateeverybytes 
agent.logging.files.keepfiles
agent.logging.files.interval
```

<img width="1237" alt="image"
src="https://github.com/user-attachments/assets/8de9023c-29a0-4ecf-803a-d8c0c4b87616">

This PR also does some clean up on the default values for all these
configured advanced settings so that when user has not touched them, the
default values do not get written into the agent policy saved object.
[More info
here](elastic#200274 (comment)).

It also fixes adds missing response schemas for the advanced settings.

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
## Summary

Resolves [elastic#192237](elastic#192237).
This PR exposes the following Elastic Agent file logging configuration
options in the agent policy advanced settings UI:

```
agent.logging.to_files
agent.logging.files.rotateeverybytes 
agent.logging.files.keepfiles
agent.logging.files.interval
```

<img width="1237" alt="image"
src="https://github.com/user-attachments/assets/8de9023c-29a0-4ecf-803a-d8c0c4b87616">

This PR also does some clean up on the default values for all these
configured advanced settings so that when user has not touched them, the
default values do not get written into the agent policy saved object.
[More info
here](elastic#200274 (comment)).

It also fixes adds missing response schemas for the advanced settings.

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:feature Makes this part of the condensed release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet] Exposing logging related configuration in the Agent Policy
5 participants